Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

My/pay as bid #3

Merged
merged 23 commits into from
Aug 24, 2023
Merged

My/pay as bid #3

merged 23 commits into from
Aug 24, 2023

Conversation

m-yahya
Copy link
Contributor

@m-yahya m-yahya commented Aug 16, 2023

This will add CLI commands to call the Rust CLI.

@m-yahya m-yahya requested a review from j-ti August 17, 2023 12:51
@j-ti
Copy link
Contributor

j-ti commented Aug 21, 2023

Solves olisystems/BEST-Energy#40

Copy link
Contributor

@j-ti j-ti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works
Requested changes

  • mostly update of comments
  • adding return values e.g. in python dict type

trusted_worker_port: String,
command_name: String,
params: Vec<String>,
) -> PyResult<()> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should return value e.g. the created account string (which could be put also inside a dictionary) or the matches as Python Dict/List:
Could be similar to the following example for returning a python List of dictionaries:

    // Convert MarketOutput to Python List of Dict representing matches
    let pyMatches = PyList::new(py,
        matches.matches.into_iter().map(|mtch| {
            let dict = PyDict::new(py);
            dict.set_item("time", mtch.energy_kwh);
            dict.set_item("bid_id", mtch.bid_id);
            dict.set_item("ask_id", mtch.ask_id);
            dict.set_item("bid_actor", "");
            dict.set_item("ask_actor", "");
            dict.set_item("bid_cluster", 0);
            dict.set_item("ask_cluster", 0);
            dict.set_item("energy", mtch.energy_kwh);
            dict.set_item("price", mtch.price_euro_per_kwh);
            dict.set_item("included_grid_fee", 0);
            dict
        })
    );

As both types are different the account string or the match list could be it self returned within a dictionary with a specific key, i.e. {"account": account_string} or {"matches": pyMatches}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with the adapted algorithm, I might add some information to the matches returned by simplyr-lib both for pay as bid as well as the cluster-based matching

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created issue #5 can be changed in subsequent PR

Copy link
Contributor

@j-ti j-ti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trusted_worker_port: String,
command_name: String,
params: Vec<String>,
) -> PyResult<()> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created issue #5 can be changed in subsequent PR

@j-ti j-ti merged commit 3b0f156 into main Aug 24, 2023
@j-ti j-ti deleted the my/pay-as-bid branch August 24, 2023 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants